Skip to content

[NFC][TableGen] Minor code cleanup in SearchableTableEmitter #147856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 10, 2025

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Jul 9, 2025

  • Add braces around if/else bodies per LLVM coding standards.
  • Use range for loops and structured bindings.
  • use auto for variables initialized with dyn_cast.
  • Refactor compareBy to also use early return in the comparison loop by extracting the comparison into lambdas.

@jurahul jurahul marked this pull request as ready for review July 10, 2025 02:08
@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2025

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes
  • Add braces around if/else bodies per LLVM coding standards.
  • Use range for loops.
  • use auto for variables initialized with dyn_cast.

Full diff: https://github.com/llvm/llvm-project/pull/147856.diff

1 Files Affected:

  • (modified) llvm/utils/TableGen/SearchableTableEmitter.cpp (+24-24)
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 38fc1ee5e4020..e47e683794029 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -31,9 +31,9 @@ using namespace llvm;
 #define DEBUG_TYPE "searchable-table-emitter"
 
 static int64_t getAsInt(const Init *B) {
-  if (const BitsInit *BI = dyn_cast<BitsInit>(B))
+  if (const auto *BI = dyn_cast<BitsInit>(B))
     return *BI->convertInitializerToInt();
-  if (const IntInit *II = dyn_cast<IntInit>(B))
+  if (const auto *II = dyn_cast<IntInit>(B))
     return II->getValue();
   llvm_unreachable("Unexpected initializer");
 }
@@ -115,20 +115,20 @@ class SearchableTableEmitter {
 
   std::string primaryRepresentation(SMLoc Loc, const GenericField &Field,
                                     const Init *I) {
-    if (const StringInit *SI = dyn_cast<StringInit>(I)) {
+    if (const auto *SI = dyn_cast<StringInit>(I)) {
       if (Field.IsCode || SI->hasCodeFormat())
         return SI->getValue().str();
       else
         return SI->getAsString();
-    } else if (const BitsInit *BI = dyn_cast<BitsInit>(I))
+    } else if (const auto *BI = dyn_cast<BitsInit>(I)) {
       return "0x" + utohexstr(getAsInt(BI));
-    else if (const BitInit *BI = dyn_cast<BitInit>(I))
+    } else if (const auto *BI = dyn_cast<BitInit>(I)) {
       return BI->getValue() ? "true" : "false";
-    else if (Field.IsIntrinsic)
+    } else if (Field.IsIntrinsic) {
       return "Intrinsic::" + getIntrinsic(I).EnumName.str();
-    else if (Field.IsInstruction)
+    } else if (Field.IsInstruction) {
       return I->getAsString();
-    else if (Field.Enum) {
+    } else if (Field.Enum) {
       auto *Entry = Field.Enum->EntryMap[cast<DefInit>(I)->getDef()];
       if (!Entry)
         PrintFatalError(Loc,
@@ -140,7 +140,7 @@ class SearchableTableEmitter {
   }
 
   bool isIntrinsic(const Init *I) {
-    if (const DefInit *DI = dyn_cast<DefInit>(I))
+    if (const auto *DI = dyn_cast<DefInit>(I))
       return DI->getDef()->isSubClassOf("Intrinsic");
     return false;
   }
@@ -162,7 +162,7 @@ class SearchableTableEmitter {
       if (Ctx == TypeInTempStruct)
         return "std::string";
       return "StringRef";
-    } else if (const BitsRecTy *BI = dyn_cast<BitsRecTy>(Field.RecType)) {
+    } else if (const auto *BI = dyn_cast<BitsRecTy>(Field.RecType)) {
       unsigned NumBits = BI->getNumBits();
       if (NumBits <= 8)
         return "uint8_t";
@@ -178,8 +178,9 @@ class SearchableTableEmitter {
                                      "' of type bits is too large");
     } else if (isa<BitRecTy>(Field.RecType)) {
       return "bool";
-    } else if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction)
+    } else if (Field.Enum || Field.IsIntrinsic || Field.IsInstruction) {
       return "unsigned";
+    }
     PrintFatalError(Index.Loc,
                     Twine("In table '") + Table.Name + "' lookup method '" +
                         Index.Name + "', key field '" + Field.Name +
@@ -346,8 +347,8 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
 
     std::vector<std::pair<const Record *, unsigned>> Entries;
     Entries.reserve(Table.Entries.size());
-    for (unsigned i = 0; i < Table.Entries.size(); ++i)
-      Entries.emplace_back(Table.Entries[i], i);
+    for (auto [Idx, TblEntry] : enumerate(Table.Entries))
+      Entries.emplace_back(TblEntry, Idx);
 
     llvm::stable_sort(Entries,
                       [&](const std::pair<const Record *, unsigned> &LHS,
@@ -356,19 +357,19 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
                       });
 
     IndexRowsStorage.reserve(Entries.size());
-    for (const auto &Entry : Entries) {
-      IndexRowsStorage.push_back(Entry.first);
+    for (const auto &[EntryRec, EntryIndex] : Entries) {
+      IndexRowsStorage.push_back(EntryRec);
 
       OS << "    { ";
       ListSeparator LS;
       for (const auto &Field : Index.Fields) {
         std::string Repr = primaryRepresentation(
-            Index.Loc, Field, Entry.first->getValueInit(Field.Name));
+            Index.Loc, Field, EntryRec->getValueInit(Field.Name));
         if (isa<StringRecTy>(Field.RecType))
           Repr = StringRef(Repr).upper();
         OS << LS << Repr;
       }
-      OS << ", " << Entry.second << " },\n";
+      OS << ", " << EntryIndex << " },\n";
     }
 
     OS << "  };\n\n";
@@ -385,8 +386,8 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
        Index.Fields[0].IsInstruction)) {
     int64_t FirstKeyVal = getNumericKey(Index, IndexRows[0]);
     IsContiguous = true;
-    for (unsigned i = 0; i < IndexRows.size(); ++i) {
-      if (getNumericKey(Index, IndexRows[i]) != (FirstKeyVal + i)) {
+    for (const auto &[Idx, IndexRow] : enumerate(IndexRows)) {
+      if (getNumericKey(Index, IndexRow) != FirstKeyVal + (int64_t)Idx) {
         IsContiguous = false;
         break;
       }
@@ -496,9 +497,9 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
       OS << " ||\n      Key." << Field.Name << " != Idx->" << Field.Name;
   }
 
-  if (ShouldReturnRange)
+  if (ShouldReturnRange) {
     OS << "  return llvm::make_range(It.first, It.second);\n";
-  else if (IsPrimary) {
+  } else if (IsPrimary) {
     OS << ")\n    return nullptr;\n\n";
     OS << "  return &*Idx;\n";
   } else {
@@ -544,8 +545,7 @@ void SearchableTableEmitter::emitGenericTable(const GenericTable &Table,
 
   // The primary data table contains all the fields defined for this map.
   OS << "constexpr " << Table.CppTypeName << " " << Table.Name << "[] = {\n";
-  for (unsigned i = 0; i < Table.Entries.size(); ++i) {
-    const Record *Entry = Table.Entries[i];
+  for (const auto &[Idx, Entry] : enumerate(Table.Entries)) {
     OS << "  { ";
 
     ListSeparator LS;
@@ -554,7 +554,7 @@ void SearchableTableEmitter::emitGenericTable(const GenericTable &Table,
          << primaryRepresentation(Table.Locs[0], Field,
                                   Entry->getValueInit(Field.Name));
 
-    OS << " }, // " << i << "\n";
+    OS << " }, // " << Idx << "\n";
   }
   OS << " };\n";
 

@jurahul jurahul requested a review from topperc July 10, 2025 16:04
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

jurahul added 5 commits July 10, 2025 13:11
- Add braces around if/else bodies per LLVM coding standards.
- Use range for loops.
- use auto for variables initialized with `dyn_cast`.
@jurahul jurahul force-pushed the cc_searchable_table_emitter branch from 8887292 to 1b87517 Compare July 10, 2025 20:17
@jurahul jurahul merged commit b6a4621 into llvm:main Jul 10, 2025
9 checks passed
@jurahul jurahul deleted the cc_searchable_table_emitter branch July 10, 2025 21:52
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 10, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/23251

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: tools/llvm-lipo/archs-universal-binary-arm.test' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
/Users/buildbot/buildbot-root/aarch64-darwin/build/bin/yaml2obj /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/test/tools/llvm-lipo/archs-universal-binary-arm.test -o /Users/buildbot/buildbot-root/aarch64-darwin/build/test/tools/llvm-lipo/Output/archs-universal-binary-arm.test.tmp # RUN: at line 1
+ /Users/buildbot/buildbot-root/aarch64-darwin/build/bin/yaml2obj /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/test/tools/llvm-lipo/archs-universal-binary-arm.test -o /Users/buildbot/buildbot-root/aarch64-darwin/build/test/tools/llvm-lipo/Output/archs-universal-binary-arm.test.tmp
yaml2obj: error: failed to open '/Users/buildbot/buildbot-root/aarch64-darwin/build/test/tools/llvm-lipo/Output/archs-universal-binary-arm.test.tmp': No space left on device

--

********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants